Skip to content

Conversation

JarredAllen
Copy link
Contributor

changelog: Expand the needless_collect lint as suggested in #5627 (WIP).

This PR is WIP because I can't figure out how to make the multi-part suggestion include its changes in the source code (the fixed is identical to the source, despite the lint making suggestions). Aside from that one issue, I think this should be good.

@rust-highfive
Copy link

r? @phansch

(rust_highfive has picked a reviewer for you, use r? to override)

@rust-highfive rust-highfive added the S-waiting-on-review Status: Awaiting review from the assignee but also interested parties label Jul 23, 2020
Copy link
Contributor

@phansch phansch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

how to make the multi-part suggestion include its changes in the source code

Multi-part suggestions are not yet supported by rustfix, so that's not something we could fix here.

Minus the nits, this looks good to me, thanks!

@@ -0,0 +1,22 @@
// run-rustfix

#[allow(unused)]
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Did you mean this to be #![allow(unused)]?

let is_empty = sym!(is_empty);
let contains = sym!(contains);
match method_name.ident.name {
name if name == into_iter => self.uses.push(
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ah.. function calls are not allowed in patterns. At least for into_iter we can use the pre-interned symbol from rustc (will need a use rustc_span::symbol::sym):

https://github.com/rust-lang/rust/blob/46cf80dc1a75ad27f67e79f73fec371a16762494/src/librustc_span/symbol.rs#L594

Suggested change
name if name == into_iter => self.uses.push(
sym::into_iter => self.uses.push(

if let Some(ref generic_args) = method_name.args;
if let Some(GenericArg::Type(ref ty)) = generic_args.args.get(0);
if let ty = cx.typeck_results().node_type(ty.hir_id);
if is_type_diagnostic_item(cx, ty, sym!(vec_type)) ||
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
if is_type_diagnostic_item(cx, ty, sym!(vec_type)) ||
if is_type_diagnostic_item(cx, ty, sym::vec_type) ||

@JarredAllen JarredAllen changed the title WIP needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ... needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ... Aug 3, 2020
@JarredAllen
Copy link
Contributor Author

@phansch I think I've added those suggestions.

Copy link
Contributor

@phansch phansch left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

LGTM, thanks again!

@flip1995 flip1995 added S-waiting-on-bors Status: The marked PR was approved and is only waiting bors and removed S-waiting-on-review Status: Awaiting review from the assignee but also interested parties labels Aug 3, 2020
@flip1995
Copy link
Member

flip1995 commented Aug 4, 2020

@bors r=phansch rollup=always

@bors
Copy link
Contributor

bors commented Aug 4, 2020

📌 Commit 5e10b03 has been approved by phansch

flip1995 added a commit to flip1995/rust-clippy that referenced this pull request Aug 4, 2020
needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ...

changelog: Expand the needless_collect lint as suggested in rust-lang#5627 (WIP).

This PR is WIP because I can't figure out how to make the multi-part suggestion include its changes in the source code (the fixed is identical to the source, despite the lint making suggestions). Aside from that one issue, I think this should be good.
bors added a commit that referenced this pull request Aug 4, 2020
Rollup of 6 pull requests

Successful merges:

 - #5725 (should_impl_trait - ignore methods with lifetime params)
 - #5837 (needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ...)
 - #5846 (Handle mapping to Option in `map_flatten` lint)
 - #5848 (Add derive_ord_xor_partial_ord lint)
 - #5852 (Add lint for duplicate methods of trait bounds)
 - #5856 (Remove old Symbol reexport)

Failed merges:

r? @ghost

changelog: rollup
bors added a commit that referenced this pull request Aug 4, 2020
Rollup of 6 pull requests

Successful merges:

 - #5725 (should_impl_trait - ignore methods with lifetime params)
 - #5837 (needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ...)
 - #5846 (Handle mapping to Option in `map_flatten` lint)
 - #5848 (Add derive_ord_xor_partial_ord lint)
 - #5852 (Add lint for duplicate methods of trait bounds)
 - #5856 (Remove old Symbol reexport)

Failed merges:

r? @ghost

changelog: rollup
bors added a commit that referenced this pull request Aug 4, 2020
Rollup of 5 pull requests

Successful merges:

 - #5837 (needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ...)
 - #5846 (Handle mapping to Option in `map_flatten` lint)
 - #5848 (Add derive_ord_xor_partial_ord lint)
 - #5852 (Add lint for duplicate methods of trait bounds)
 - #5856 (Remove old Symbol reexport)

Failed merges:

r? @ghost
bors added a commit that referenced this pull request Aug 4, 2020
Rollup of 5 pull requests

Successful merges:

 - #5837 (needless_collect: catch x: Vec<_> = iter.collect(); x.into_iter() ...)
 - #5846 (Handle mapping to Option in `map_flatten` lint)
 - #5848 (Add derive_ord_xor_partial_ord lint)
 - #5852 (Add lint for duplicate methods of trait bounds)
 - #5856 (Remove old Symbol reexport)

Failed merges:

r? @ghost

changelog: rollup
@bors bors merged commit ca2a25d into rust-lang:master Aug 4, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
S-waiting-on-bors Status: The marked PR was approved and is only waiting bors
Projects
None yet
Development

Successfully merging this pull request may close these issues.

5 participants